-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat(kubernetes_logs source) support k8s API based logs #24007
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
@pront , I have introduced the new config named Hope that the whole strucuture is good, and then we could focus on the temporary workaround to handle the |
| return; | ||
| // TODO: fix it until `FunctionTransform` supports Api logs | ||
| // emit!(ParserMatchError { value: &s[..] }); | ||
| drop(log.insert(&message_path, Value::Bytes(s))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is a ugly workaround. For more reasonable PR review size, I dropped the implementation of FunctionTransform trait for Api here (which is very similar to how Cri handle the logs).
| /// The strategy to use for log collection. | ||
| log_collection_strategy: LogCollectionStrategy, | ||
| } | ||
|
|
||
| /// Configuration for the log collection strategy. | ||
| #[configurable_component] | ||
| #[derive(Clone, Copy, Debug, PartialEq, Eq)] | ||
| #[serde(rename_all = "snake_case")] | ||
| enum LogCollectionStrategy { | ||
| /// Collect logs by reading log files from the filesystem. | ||
| File, | ||
| /// Collect logs via the Kubernetes Logs API. | ||
| Api, | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a default value in order not to break existing users. I think this works:
| /// The strategy to use for log collection. | |
| log_collection_strategy: LogCollectionStrategy, | |
| } | |
| /// Configuration for the log collection strategy. | |
| #[configurable_component] | |
| #[derive(Clone, Copy, Debug, PartialEq, Eq)] | |
| #[serde(rename_all = "snake_case")] | |
| enum LogCollectionStrategy { | |
| /// Collect logs by reading log files from the filesystem. | |
| File, | |
| /// Collect logs via the Kubernetes Logs API. | |
| Api, | |
| } | |
| /// The strategy to use for log collection. | |
| #[serde(default)] | |
| log_collection_strategy: LogCollectionStrategy, | |
| } | |
| /// Configuration for the log collection strategy. | |
| #[configurable_component] | |
| #[derive(Clone, Copy, Debug, PartialEq, Eq)] | |
| #[serde(rename_all = "snake_case")] | |
| enum LogCollectionStrategy { | |
| /// Collect logs by reading log files from the filesystem. | |
| #[default] | |
| File, | |
| /// Collect logs via the Kubernetes Logs API. | |
| Api, | |
| } |
You also need to run make generate-component-docs in order to generate documentation for this and then we can see if it all looks ok there too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/sources/kubernetes_logs/mod.rs
Outdated
| let reflector_stream = BroadcastStream::new(reflector_rx).filter_map(|result| { | ||
| ready(match result { | ||
| Ok(event) => Some(Ok(event)), | ||
| Err(_) => None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Silently dropping the error here is a bad idea. I have recently come across an issue (#24220) where a broadcast channel is erroring due to RecvError::Lagged. This will silently drop logs.
I am weary of using a broadcast channel here for the same reason as listed in that issue. IMO a remediation needs to be explored for 24220 and the same strategy needs to be applied here
src/sources/kubernetes_logs/mod.rs
Outdated
| log_namespace, | ||
| ); | ||
|
|
||
| // TODO: annotate the logs with pods's metadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // TODO: annotate the logs with pods's metadata | |
| // TODO: annotate the logs with pods's metadata. Need to honor the `insert_namespace_fields` flag |
Minor addition that can help guide the future implementation
|
|
||
| // Process the stream by reading line by line | ||
| loop { | ||
| match log_stream.read_until(b'\n', &mut buffer).await { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks very similar to read_until_with_max_size from lib/file-source-common/src/buffer.rs. Maybe That can be used here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The log_stream implements futures::AsyncBufRead , but the read_until_with_max_size expects a tokio::io::AsyncBufRead. Unless we use the async-compact crate, it seems that it's not easy to cooperate it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to change then, this was an optional suggestion. I'll take a look myself at this later
| let line = Line { | ||
| text: line_bytes, | ||
| filename: String::new(), // Filename is not applicable for k8s logs | ||
| file_id: FileFingerprint::FirstLinesChecksum(0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not correct and will likely cause issues with the file source implementation due to referencing files with the same checksum. Maybe a new fingerprint type here is warranted
Signed-off-by: titaneric <[email protected]>
Signed-off-by: titaneric <[email protected]>
Signed-off-by: titaneric <[email protected]>
Signed-off-by: titaneric <[email protected]>
Signed-off-by: titaneric <[email protected]>

Summary
As dicussed in #23597 and #23982, I intend to support Kubernetes logs API to tail the pods logs in this PR.
Features Implemented
log_collection_strategyconfiguration option to enable log collection via Kubernetes API instead of file-based tailingPodInfostruct for essential pod metadata extraction and container trackingTODO Items
Vector configuration
How did you test this PR?
Given the following kind cluster config named
kind-cluster.yamlCreate kind clulster from config
Export the cluster's kubeconfig to
kind-kubeconfigkind export kubeconfig --kubeconfig kind-kubeconfigApply the following manifest to kind's cluster
Build vector with nessary features and run the given config.
Change Type
Is this a breaking change?
Does this PR include user facing changes?
no-changeloglabel to this PR.References
Notes
@vectordotdev/vectorto reach out to us regarding this PR.pre-pushhook, please see this template.make fmtmake check-clippy(if there are failures it's possible some of them can be fixed withmake clippy-fix)make testgit merge origin masterandgit push.Cargo.lock), pleaserun
make build-licensesto regenerate the license inventory and commit the changes (if any). More details here.